feat(authbridge): Wire mtlsMode through to per-agent config#369
Conversation
Introduces a MTLSMode field on AgentRuntimeSpec and plumbs it through the existing CR > namespace > default resolution chain so the admission webhook can render an mtls: block into the per-agent authbridge ConfigMap. With this in place, kagenti-extensions PR rossoctl#424 (mTLS between authbridge sidecars on the proxy-sidecar / lite paths) can be enabled declaratively per workload instead of by hand-editing the namespace ConfigMap. Behavior: * MTLSMode enum: disabled (default) | permissive | strict. Resolution: AgentRuntime CR > namespace authbridge-runtime-config mtls.mode > "disabled". Mirrors the existing AuthBridgeMode chain. * Per-agent ConfigMap renders top-level mtls: {mode: <value>} when resolved mode is non-disabled. Cert paths are intentionally not emitted — they default to authbridge's bundled-spiffe-helper convention (/opt/svid.pem, /opt/svid_key.pem, /opt/svid_bundle.pem), so surfacing them here would couple the operator to authbridge's internal layout for no benefit. Stale mtls blocks in the base YAML are scrubbed when toggling back to disabled. * mtlsMode != disabled implicitly requires SPIRE because the bundled spiffe-helper writes the X.509 SVID files mTLS reads. The pod mutator auto-enables SPIRE for the workload when mtlsMode is set — operators don't need to add the spiffe-helper-inject label separately. The operator does not mutate the namespace authbridge-config ConfigMap (which would race with the kagenti meta-chart for ownership); it just changes the in-memory injection decision. * The AgentRuntime validating webhook rejects mtlsMode != disabled when authBridgeMode is envoy-sidecar. Envoy SDS isn't currently configured by the kagenti envoy-config, so accepting the combo would silently produce a workload running plaintext on the wire while the user believed they had strict mTLS. The error message points to the supported modes (proxy-sidecar / lite) and flags this as a current limitation, not a permanent one — when the envoy-config gains SDS support, the rejection just goes away. * Spec changes to AuthBridgeMode and MTLSMode now flow into the AgentRuntime controller's resolved-config hash. Editing either field re-stamps the kagenti.io/config-hash pod-template annotation and the Deployment rolls automatically. (Pre-existing gap fixed along the way: AuthBridgeMode changes also weren't triggering rollouts before this commit.) Out of scope: * envoy-sidecar mTLS via Envoy SDS — separate piece of work in the kagenti meta-chart's envoy-config template. * Namespace-level authbridge-runtime-config edits triggering rollouts. Today the namespace ConfigMap is consumed by the admission webhook only; namespace-level mtls.mode changes won't auto-roll existing workloads. Operators editing the namespace config should kubectl rollout restart manually until that's addressed. * Chart-side CRD at charts/kagenti-operator/crds/ is intentionally not touched. That copy has been drifting from config/crd/bases/ since commit 677f63d (predates `lite` mode entirely). Recent CRD-touching commits (4eb62af, 21a2258) follow the same "config is source of truth" pattern; this PR matches. Cleaning up the chart-side drift is a separate concern. Tests added: * ExtractMTLSMode parser cases (empty, missing block, unknown mode, malformed YAML, mtls coexisting with other top-level blocks). * ensurePerAgentConfigMap: strict and permissive emit the block; disabled and "" omit the block; stale-block scrubbing on toggle back to disabled. * Validating webhook: 11-case matrix of (mode, mtls) combinations on Create and Update; envoy-sidecar + non-disabled rejected, error message points to the follow-up. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Two pre-existing tech-debt items folded into the mtlsMode wiring PR (addressing reviewer-callout from PR rossoctl#369 itself): 1. Namespace-CM-edit rollout gap. The controller's resolved-config hash captured a few CR fields and the cluster-defaults / namespace-defaults ConfigMaps, but NOT the namespace authbridge-runtime-config ConfigMap that the admission webhook reads at pod creation. So `kubectl edit configmap authbridge-runtime-config -n team1` silently failed to roll the workloads in that namespace — operators had to know to run `kubectl rollout restart` manually. - Add an AuthBridgeRuntime field to resolvedConfig that captures the raw config.yaml content (not parsed — any byte change rolls the workload, agnostic to whether it's mtls / pipeline / listener edits). - Extend mapNamespaceConfigMapToAgentRuntimes to also match ConfigMaps named "authbridge-runtime-config" (not just kagenti.io/defaults=true labeled ones), so edits to that CM enqueue every AgentRuntime in the namespace and the hash re-stamps the pod template. - New constant AuthBridgeRuntimeConfigMapName for the magic name. Other places in the codebase still use the literal string; consolidating them is out of scope. - Tests: hash-changes-on-edit (extends ComputeConfigHash spec) + mapper-matches-by-name (extends the existing namespace mapper test). 2. Chart-CRD drift. charts/kagenti-operator/crds/ holds a manually-maintained Helm crds/ directory copy of the AgentRuntime + AgentCard CRDs. It was last touched in commit 677f63d (initial webhook migration) and has been silently drifting from kagenti-operator/config/crd/bases/ for at least three subsequent CRD-touching commits (21a2258, 4eb62af, and earlier this PR). Anyone helm-installing got an outdated CRD missing recent fields like authBridgeMode / lite mode / mtlsMode. - Sync both CRDs from config/crd/bases/ to charts/.../crds/. - Add a sync-chart-crds Makefile target invoked at the tail of `make manifests`, so future controller-gen regenerations propagate to the chart copy automatically. Skips the stub `_.yaml` controller-gen sometimes emits. PR rossoctl#369 originally noted both items as out-of-scope follow-ups; this commit folds them in per reviewer/maintainer feedback. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
goconst flagged the new literal-"true" comparison in mapNamespaceConfigMapToAgentRuntimes, suggesting we reuse AnnotationRestartPendingValue — but that constant is semantically a restart-pending marker, not a generic label-true value. Reusing it would obscure intent. Existing code (defaults_config_reconciler.go:283 etc.) uses the same literal-true idiom for label-truth checks; the rule fires because --new-from-patch only inspects added lines. Suppress on the single line with a comment explaining the reasoning, rather than introducing a fresh labelValueTrue constant only here. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Four review items folded in: * rossoctl#2 Document the burst-reconcile implication of byte-hashing the namespace authbridge-runtime-config CM. Single-edit on N agents re-hashes all N pod specs; Kubernetes sequences the actual rolls but the controller load fans out. Fine for typical small namespaces; worth knowing before formatting edits during peak. * rossoctl#3 Add CR-side hash-change tests parallel to the existing CM-edit test: flipping MTLSMode disabled→strict on the AgentRuntime CR re-hashes; flipping AuthBridgeMode proxy-sidecar→lite on the CR re-hashes. Locks the rollout-on-CR-edit behavior so a future refactor that drops MTLSMode / AuthBridgeMode from resolvedConfig silently regresses into a failing test, not a silent shipping bug. * rossoctl#5 Clarify the MTLSMode field godoc on CR=empty vs CR="disabled". These ARE observably different in `kubectl get -o yaml` and now produce different effective behavior: - empty: falls through to namespace ConfigMap, then to default. - "disabled": pinned at the CR layer; namespace cannot override. The original resolver had a bug: both fell through to the namespace because the fallthrough check used `mtlsMode == MTLSModeDisabled` rather than "did the CR set the field." Fix the resolver to use a sentinel ("" = unset) — same shape as the AuthBridgeMode resolver above. Without this fix, the godoc would have been documenting unimplemented behavior. * rossoctl#6 Add a comment on the unrecognized-mtlsMode WARN explaining the defense-in-depth intent (CRD enum check at the API server is the primary line; this is insurance against a future schema-validation regression and against non-CRD config sources). Prevents a future cleanup PR from dropping it as "redundant." rossoctl#1 (auto-SPIRE log clarity) is already covered by the existing mutatorLog.Info("mtlsMode set; auto-enabling SPIRE for this workload", ...) — the why is in the message itself. No change. rossoctl#4 (stale-mtls-block scrub end-to-end test) is already covered by TestEnsurePerAgentConfigMap_MTLSScrubsStaleBlock — that test exactly exercises "base YAML has stale mtls: strict, render with MTLSModeDisabled, assert block gone." Reply on the PR pointing the reviewer at it. rossoctl#7 was an observation, no change. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
pdettori
left a comment
There was a problem hiding this comment.
Solid PR — well-structured mTLS wiring with proper resolution chain, auto-SPIRE, and defensive validation. The namespace-CM rollout fix and chart-CRD sync are welcome bonuses.
Areas reviewed: Go (API types, controller, webhook, pod mutator), Helm/CRD, Makefile
Commits: 4 commits, all signed-off
CI: 15/15 passing
No blocking issues. Test coverage is thorough and the code follows existing patterns cleanly.
pdettori
left a comment
There was a problem hiding this comment.
Clean, well-structured PR that adds mTLS mode support to AgentRuntime with a clear three-layer resolution chain (CR → namespace CM → default). The design choices are sound: auto-enabling SPIRE when mTLS is needed, rejecting the incompatible envoy-sidecar combo at admission, and properly distinguishing unset vs explicit-disabled. Test coverage is comprehensive across all four concerns (hash, extraction, validation, rendering).
Areas reviewed: Go source, Go tests, Helm CRDs, Makefile
Commits: 4 commits, all signed-off ✓
CI status: all 15 checks passing ✓
No security issues found.
| // defaults_config_reconciler.go) uses the same literal-true idiom | ||
| // for label checks; rather than introduce a fresh `labelValueTrue` | ||
| // constant only here, suppress the rule on this one line. | ||
| isNsDefaults := labels[LabelNamespaceDefaults] == "true" //nolint:goconst |
There was a problem hiding this comment.
nit: The goconst suppression is well-justified. Consider extracting a const labelValueTrue = "true" in constants.go in a future cleanup pass if this pattern recurs — but agree it's not worth it for one occurrence.
Summary
Wires the now-merged kagenti-extensions PR #424 (mTLS between authbridge sidecars on the proxy-sidecar / lite paths) through the operator so it can be enabled declaratively per workload — and folds in two pre-existing tech-debt items along the way.
The mTLS feature is already fully implemented on the authbridge side: the byte-peek listener, X.509 SVID consumption, permissive-with-fallback / strict modes — all live in the merged PR. This PR makes the operator render the right config, reject impossible combos, and closes two related rollout / chart-sync gaps.
What ships
mtlsMode wiring (commit 1)
AgentRuntimeSpec.MTLSMode— new enum field (disabled|permissive|strict), defaultdisabled. Resolution chain mirrorsAuthBridgeMode: CR > namespaceauthbridge-runtime-configmtls.mode> default.Per-agent ConfigMap renders
mtls: {mode: <value>}when resolved mode is non-disabled. Cert paths intentionally omitted — they match authbridge's bundled-spiffe-helper defaults (/opt/svid.pemetc.). Stalemtlsblocks scrubbed when toggling back to disabled.mtlsMode != disabledauto-enables SPIRE for the workload (setsspireEnabledfor that injection + ensures the dedicated ServiceAccount). Operators don't need to add thekagenti.io/spiffe-helper-injectlabel separately. We don't mutate the namespaceauthbridge-configConfigMap — that would race with the kagenti meta-chart for ownership.Validating webhook rejects
mtlsMode != disabled+authBridgeMode = envoy-sidecar. Envoy SDS isn't currently configured by the kagenti envoy-config. Accepting that combo would silently run plaintext while the user believed they had strict mTLS — exactly the wrong failure mode for a security-relevant posture claim. Error message names the supported modes (proxy-sidecar/lite) and explicitly flags this as a current limitation, not a permanent one.Spec changes to
AuthBridgeModeandMTLSModefeed the resolved-config hash. Editing either CR field re-stampskagenti.io/config-hashon the pod template, so the Deployment rolls automatically. (Bonus: this fixes a pre-existing gap whereAuthBridgeModechanges weren't triggering rollouts.)Namespace-CM-edit rollout gap (commit 2)
The controller's resolved-config hash captured CR fields and cluster/namespace-defaults ConfigMaps, but not the
authbridge-runtime-configConfigMap that the admission webhook reads at pod creation. Editing it silently failed to roll workloads — operators had to know tokubectl rollout restartmanually.AuthBridgeRuntimefield onresolvedConfigcaptures the rawconfig.yamlcontent (not parsed — any byte change rolls the workload, agnostic to whether it's mtls / pipeline / listener edits).mapNamespaceConfigMapToAgentRuntimesextended to also match ConfigMaps namedauthbridge-runtime-config(not justkagenti.io/defaults=truelabeled ones).Chart-CRD drift (commit 2)
charts/kagenti-operator/crds/was last touched in commit 677f63d (initial webhook migration) and has been silently drifting fromconfig/crd/bases/for at least three subsequent CRD-touching commits (21a2258, 4eb62af, this PR). Anyone helm-installing got an outdated CRD missing recent fields likeauthBridgeMode,litemode,mtlsMode.agent.kagenti.dev_agentruntimes.yamlandagent.kagenti.dev_agentcards.yamlfromconfig/crd/bases/.sync-chart-crdsMakefile target invoked at the tail ofmake manifests. Futuremake manifestsruns propagate to the chart copy automatically.Out of scope (genuine, smaller follow-ups)
tls_inspector, transport_socket on listeners). Comparable in size to PR Fix: harden controllers against stale objects, orphaned resources, an… #424 itself.Test plan
go test -race -count=1 ./internal/... ./api/...— all passgo vet ./...,gofmt -lcleanmake manifestsregenerates CRD schema with newmtlsModeenum AND propagates to chart copymake sync-chart-crdsis idempotent (running it twice produces no diff)mtlsMode: strictagainst a kagenti-extensions build that includes PR Fix: harden controllers against stale objects, orphaned resources, an… #424, confirm wire-level handshake, then patch topermissiveand confirm the rolling update fireskubectl edit configmap authbridge-runtime-config -n team1addingmtls.mode: permissive, confirm Deployments in that namespace roll automaticallyFiles
Commit 1 (mtlsMode wiring):
api/v1alpha1/agentruntime_types.go— newMTLSModefield with godocconfig/crd/bases/agent.kagenti.dev_agentruntimes.yaml— regenerated schemainternal/webhook/injector/agentruntime_config.go— extract from CRinternal/webhook/injector/namespace_config.go—ExtractMTLSModeparserinternal/webhook/injector/constants.go—MTLSModeDisabled/Permissive/Strictinternal/webhook/injector/pod_mutator.go— resolution chain + render + auto-SPIREinternal/webhook/v1alpha1/agentruntime_webhook.go—checkMTLSCompatibleWithModeinternal/controller/agentruntime_config.go— fold AuthBridgeMode + MTLSMode into hashCommit 2 (rollout gap + chart drift):
internal/controller/agentruntime_config.go—AuthBridgeRuntimefield on resolvedConfiginternal/controller/agentruntime_controller.go— extend mapper forauthbridge-runtime-configkagenti-operator/Makefile—sync-chart-crdstargetcharts/kagenti-operator/crds/agent.kagenti.dev_agentruntimes.yaml— syncedcharts/kagenti-operator/crds/agent.kagenti.dev_agentcards.yaml— syncedNet: ~675 LOC added across 13 files (most of it tests + comments + generated CRD content).
Assisted-By: Claude (Anthropic AI) noreply@anthropic.com